V2 fix #1626: Ошибки и несоответствия методов КоллекцииКолонокТаблицыЗначений by Mr-Rm · Pull Request #1628 · EvilBeaver/OneScript

Mr-Rm · 2025-12-06T21:13:05Z

Изменения в ContextValuesMarshaller имеют влияние на весь движок. Все тесты прошли, gitsync, winow работают.

Summary by CodeRabbit

Bug Fixes
- Added validation for column names; invalid identifiers are now rejected with an error instead of being silently accepted.
- Strengthened type compatibility checking for value conversions to prevent incompatible types from being accepted.
Tests
- Added comprehensive test coverage for invalid column name handling and type validation scenarios.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

coderabbitai · 2025-12-06T21:13:14Z

Walkthrough

Added input validation for column names in ValueTableColumnCollection with exception handling, refactored methods to expression-bodied syntax, tightened type compatibility checks in ContextValuesMarshaller, and added test procedures validating exception scenarios for invalid column names and index types.

Changes

Cohort / File(s)	Summary
Column validation and refactoring `src/OneScript.StandardLibrary/Collections/ValueTable/ValueTableColumnCollection.cs`	Added column name validation in Add and Insert methods; converted FindColumnByIndex, GetIndexedValue, GetPropCount, IsPropWritable, IsPropReadable to expression-bodied form; enhanced GetColumnByIIndex with null-coalescing throw pattern; updated GetColumnNumericIndex error handling to IndexOutOfRange; applied pattern matching for type checks.
Type checking enforcement `src/ScriptEngine/Machine/Contexts/ContextValuesMarshaller.cs`	Tightened IRuntimeContextInstance type checking by validating type compatibility before casting; changed error handling to throw InvalidCastException for incompatible types instead of accepting them.
Test coverage `tests/valuetable.os`	Added four exported test procedures: ТестДолжен_ВызватьИсключениеПриДобавленииКолонкиСНевернымИменем, ТестДолжен_ВызватьИсключениеПриВставкеКолонкиСНевернымИменем, ТестДолжен_ВызватьИсключениеНаПоискеИндексаКолонкиСНевернымПримитивнымТипом, and ТестДолжен_ВызватьИсключениеНаПоискеИндексаКолонкиСНевернымОбъектнымТипом; registered in GetList.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pattern matching and expression-bodied refactoring: Verify that syntax conversions preserve original behavior, particularly in GetColumnByIIndex and GetColumnNumericIndex.
Type compatibility validation: Ensure the new type checking in ContextValuesMarshaller.cs correctly enforces constraints without breaking existing compatible casts.
Error path changes: Confirm InvalidCastException and ColumnException.WrongColumnName are appropriate and properly propagate to callers.
Test procedure names: Review that test procedures adequately cover the new validation paths and edge cases.

Possibly related PRs

fix #1582: сравнение примитивных типов #1584: Modifies ValueTable column error handling and ColumnException usage, directly related to column name validation logic.
V2 #1580: Ошибки и несоответствия разных методов ТаблицыЗначений #1623: Modifies ValueTableColumnCollection implementation (same file), affected by structural refactoring.

Suggested reviewers

EvilBeaver

Poem

🐰 Columns now checked with care,
Names validated with flair,
Types aligned so neat,
Tests make the change complete—
Refactored code, cleaner everywhere! ✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 18.75% which is insufficient. The required threshold is 80.00%.	You can run `@coderabbitai generate docstrings` to improve docstring coverage.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title clearly references a specific issue (#1626) and describes the main change: fixing errors and inconsistencies in the ValueTableColumnCollection methods, which aligns with the actual changes made to that class and related components.

✨ Finishing touches

📝 Generate docstrings

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 90cb7e7 and 447af44.

📒 Files selected for processing (3)

src/OneScript.StandardLibrary/Collections/ValueTable/ValueTableColumnCollection.cs (8 hunks)
src/ScriptEngine/Machine/Contexts/ContextValuesMarshaller.cs (1 hunks)
tests/valuetable.os (2 hunks)

🧰 Additional context used

📓 Path-based instructions (1)

**/*.cs

📄 CodeRabbit inference engine (.cursor/rules/langversion.mdc)

Do not use C# language features that are not available in C# 8 when generating code for projects using .NET 8.0 with LangVersion 8

Files:

src/ScriptEngine/Machine/Contexts/ContextValuesMarshaller.cs
src/OneScript.StandardLibrary/Collections/ValueTable/ValueTableColumnCollection.cs

🧠 Learnings (2)

📚 Learning: 2024-08-29T06:48:46.675Z

Learnt from: Mr-Rm
Repo: EvilBeaver/OneScript PR: 1447
File: src/ScriptEngine/Machine/ValueFactory.cs:102-104
Timestamp: 2024-08-29T06:48:46.675Z
Learning: When performing operations that involve type conversion, ensure that the conversion function (e.g., `AsNumber`) handles invalid cases by throwing an appropriate exception, rather than adding redundant type checks in the calling method.

Applied to files:

src/ScriptEngine/Machine/Contexts/ContextValuesMarshaller.cs

📚 Learning: 2025-12-05T07:24:02.341Z

Learnt from: EvilBeaver
Repo: EvilBeaver/OneScript PR: 1623
File: src/OneScript.StandardLibrary/Collections/ValueTable/ValueTable.cs:84-94
Timestamp: 2025-12-05T07:24:02.341Z
Learning: In OneScript's ValueTable.Insert method (src/OneScript.StandardLibrary/Collections/ValueTable/ValueTable.cs), when the index parameter is negative or greater than the row count (index < 0 || index > _rows.Count), the new row should be appended to the end of the table. This behavior matches 1C platform compatibility, where out-of-bounds insertion adds rows to the end rather than throwing an exception.

Applied to files:

src/OneScript.StandardLibrary/Collections/ValueTable/ValueTableColumnCollection.cs

🧬 Code graph analysis (1)

src/OneScript.StandardLibrary/Collections/ValueTable/ValueTableColumnCollection.cs (2)

src/OneScript.Core/Commons/Utils.cs (1)

Utils (12-54)

src/OneScript.StandardLibrary/Collections/Exceptions/ColumnException.cs (7)

ColumnException (13-53)

ColumnException (15-18)

ColumnException (20-22)

ColumnException (24-29)

ColumnException (31-36)

ColumnException (38-43)

ColumnException (46-51)

🔇 Additional comments (8)

src/ScriptEngine/Machine/Contexts/ContextValuesMarshaller.cs (1)

244-250: Good type safety improvement.

This change correctly validates type compatibility before calling AsObject(), ensuring incompatible types are rejected with InvalidCastException. The exception is properly handled upstream in ConvertParam (lines 70-73) and converted to RuntimeException.InvalidArgumentType(). Based on learnings, this follows the pattern of having the conversion function handle invalid cases directly.

src/OneScript.StandardLibrary/Collections/ValueTable/ValueTableColumnCollection.cs (5)

47-59: Good input validation for column names.

Validating column names against Utils.IsValidIdentifier before adding ensures data integrity and provides clear error messages when invalid names like "!@#" are passed. The validation is correctly placed before the duplicate check and column creation.

71-83: Consistent validation with Add method.

Good to see the same validation logic applied to both Add and Insert methods, ensuring consistent behavior across the API.

159-159: LGTM - Expression-bodied member refactors.

These refactors to expression-bodied syntax improve code conciseness for single-expression methods while maintaining identical behavior.

202-225: Improved null handling and exception semantics.

Lines 206-207: The null-coalescing throw pattern is cleaner than a separate null check.

Line 214: IndexOutOfRange is more semantically accurate than InvalidArgumentValue for bounds checking.

Lines 219-221: Pattern matching with is ValueTableColumn column is safer and more readable than a cast.

227-249: Consistent improvements with GetColumnByIIndex.

The same improvements (semantically accurate IndexOutOfRange exception and pattern matching) are consistently applied here.

tests/valuetable.os (2)

81-86: Test registrations look correct.

All four new tests are properly registered. The extra blank line at lines 85-86 is a minor style inconsistency but doesn't impact functionality.

1244-1290: Comprehensive test coverage for the new validation.

The four new tests effectively cover:

Adding columns with invalid names

Inserting columns with invalid names

Passing wrong primitive type (number) to IndexOf

Passing wrong object type (ValueTable instead of ValueTableColumn) to IndexOf

The tests properly verify both that exceptions are thrown and that the error messages are correct. This aligns well with the changes in ValueTableColumnCollection.cs and ContextValuesMarshaller.cs.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Mr-Rm added 3 commits December 6, 2025 23:19

fix EvilBeaver#1626.1: проверка корректности имени добавляемой колонки

8de2f06

fix EvilBeaver#1626.2: корректное сообщение об ошибочном типе аргумента

e097ab7

скорректирован тип исключения; мекие правки

447af44

EvilBeaver approved these changes Dec 8, 2025

View reviewed changes

EvilBeaver merged commit 03b0459 into EvilBeaver:develop Dec 8, 2025
1 of 2 checks passed

coderabbitai bot mentioned this pull request Dec 8, 2025

V2 Исправления к СтрокаТаблицыЗначений #1629

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

V2 fix #1626: Ошибки и несоответствия методов КоллекцииКолонокТаблицыЗначений#1628

V2 fix #1626: Ошибки и несоответствия методов КоллекцииКолонокТаблицыЗначений#1628
EvilBeaver merged 3 commits intoEvilBeaver:developfrom
Mr-Rm:v2/fix-1626

Mr-Rm commented Dec 6, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Dec 6, 2025 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Mr-Rm commented Dec 6, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Dec 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Pre-merge checks and finishing touches

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Mr-Rm commented Dec 6, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Dec 6, 2025 •

edited

Loading